move superstructure outside of the subsystem folder #147
Conversation
Jetblackdragon
added
documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…o superstructure_change_location
|
@Jetblackdragon - I noticed you re-activated this in August. I don't know if you still need this Draft or not, but I tried to unblock you by fixing the merge. Let me know if you'd prefer I not do this and I'll revert my changes. Feel free to take it from here if you like. I also removed some dead code and fixed a broken enum. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully refactors the SuperStructure class by moving it out of the subsystems package. This is a good architectural change. The pull request also includes numerous cleanups, such as removing unused code and fixing enum access styles, which improve the overall code quality. I've identified a couple of opportunities to reduce code duplication in Controls.java by extracting repeated logic into helper methods. Addressing these would further enhance the code's maintainability.
| soloController | ||
| .leftTrigger() | ||
| .and(() -> soloScoringMode == soloScoringMode.CORAL_IN_CLAW) | ||
| .and(() -> soloScoringMode == SoloScoringMode.CORAL_IN_CLAW) | ||
| .and(() -> branchHeight != BranchHeight.CORAL_LEVEL_ONE) | ||
| .whileTrue(AutoAlign.autoAlign(s.drivebaseSubsystem, this, AutoAlign.AlignType.LEFTB)); | ||
| soloController | ||
| .leftTrigger() | ||
| .and(() -> soloScoringMode == soloScoringMode.CORAL_IN_CLAW) | ||
| .and(() -> soloScoringMode == SoloScoringMode.CORAL_IN_CLAW) | ||
| .and(() -> branchHeight == BranchHeight.CORAL_LEVEL_ONE) | ||
| .whileTrue(AutoAlign.autoAlign(s.drivebaseSubsystem, this, AutoAlign.AlignType.L1LB)); |
There was a problem hiding this comment.
This block of code for the leftTrigger is very similar to the block for the rightTrigger (lines 1035-1044). The only differences are the trigger itself (leftTrigger vs rightTrigger) and the AlignType enums used (LEFTB/L1LB vs RIGHTB/L1RB).
To improve maintainability and reduce code duplication, consider extracting this logic into a private helper method. This method could take the trigger and the alignment types as parameters.
For example:
private void configureAutoAlignForTrigger(Trigger trigger, AutoAlign.AlignType alignTypeB, AutoAlign.AlignType alignTypeL1) {
trigger
.and(() -> soloScoringMode == SoloScoringMode.CORAL_IN_CLAW)
.and(() -> branchHeight != BranchHeight.CORAL_LEVEL_ONE)
.whileTrue(AutoAlign.autoAlign(s.drivebaseSubsystem, this, alignTypeB));
trigger
.and(() -> soloScoringMode == SoloScoringMode.CORAL_IN_CLAW)
.and(() -> branchHeight == BranchHeight.CORAL_LEVEL_ONE)
.whileTrue(AutoAlign.autoAlign(s.drivebaseSubsystem, this, alignTypeL1));
}
// Then in configureSoloControllerBindings():
configureAutoAlignForTrigger(soloController.leftTrigger(), AutoAlign.AlignType.LEFTB, AutoAlign.AlignType.L1LB);
configureAutoAlignForTrigger(soloController.rightTrigger(), AutoAlign.AlignType.RIGHTB, AutoAlign.AlignType.L1RB);
No description provided.